Skip to content

fix(#470): three core gaps the P5 acceptance run hit — optional_requires, core migration ledger, scoped plugin nav (C9) - #802

Merged
Weegy merged 4 commits into
mainfrom
feat/470-c9-p5-core-gaps
Aug 20, 2026
Merged

fix(#470): three core gaps the P5 acceptance run hit — optional_requires, core migration ledger, scoped plugin nav (C9)#802
Weegy merged 4 commits into
mainfrom
feat/470-c9-p5-core-gaps

Conversation

@Weegy

@Weegy Weegy commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Fixes #795, #796, #798; epic #470 C9.

The three core gaps the P5 acceptance run of the extracted dev-platform plugin hit (byte5ai/omadia-dev-platform, docs/ACCEPTANCE-RUN-2026-08-20.md §3 G2/G3/G5). Each was measured against a real core, not inferred, and each failed in a way core itself could not see. G4 shipped as #799; G1 and G6/C12 are separate.

#795requires: had no optionality, and two gates enforced it

C2b makes ctx.services.get(name) throw for a name in neither requires: nor provides:, so a plugin must declare anything it might resolve. The installer and the boot loop then treat every requires: entry as a hard prerequisite. A plugin with a degradable dependency was therefore unrepresentable: declaring it blocked the install (409 install.missing_capability), omitting it made resolution throw.

New manifest field optional_requires:, same capability-ref syntax. It satisfies the declaration gate and neither enforcement gate — no 409, no activation hold, no provider ordered or demanded. ctx.services.getOptional(name) is the accessor that says so at the call site; it is declaration-gated exactly like get, because a typo must not quietly become undefined.

The ordering consequence is deliberate and written down rather than papered over: an optional dependency contributes no topo-sort edge, so an optional provider that is installed may activate after its consumer. An edge from a link the kernel may not enforce would turn a mutual optional reference into a cycle — a boot failure caused by a dependency the manifest declared as skippable. The contract tells plugins to resolve optional services lazily instead.

Surfaced on the install DTO (Plugin.optional_requires, plus the registry teaser) so the consent UI can render these as optional rather than as blockers. pluginServiceGrants.ts loses the paragraph calling this an open design question — every remaining legacy-allowlist row now has a manifest fix available.

#796 — the core migration ledger only ran when an LLM key was configured

middleware/migrations/ is core's own directory: 47 files including C4's plugin_public_path_grants and C7's plugin_sql_grants. Its only production caller lived inside harness-orchestrator's activate(), several hundred lines past an early return taken whenever no provider resolves. On a deployment with no key, core had no schema at all — not a degraded one, none — so recording either operator consent was structurally impossible. Silent by construction: nothing logged a migration error because no migration was ever attempted.

platform/coreMigrations.ts applies it at boot, before any tool plugin activates and independent of every provider. Ordering is load-bearing, not tidiness: ToolPluginRuntime reads a plugin's SQL-grant row while building its context, so the grant tables must exist by then. It opens its own short-lived connection from DATABASE_URL rather than waiting for graphPool — that pool is published by the knowledge-graph plugin during activation, so waiting for it would reintroduce the same defect one layer up. The orchestrator's call is retained as an explicit second pass (one SELECT against a current ledger, no lock taken) and can never be the only caller again.

#798 — no scoped plugin id could express a nav href

Core serves a plugin's bundled UI at /p/:pluginId/ui/ and Express splits on a raw /, so @scope/name resolves only percent-encoded — measured encoded 200, raw 404. HREF_SEGMENT rejects % for a good reason: the shell decides "core destinations win" by string equality, which percent-encoding defeats. The only URL that worked was the only one the validator refused, and the one it accepted 404'd.

registerNav now accepts pluginUi: true in place of a literal href and renders the canonical path itself from the id it already holds, so a plugin never hand-builds an encoded href. The literal-href validator is untouched — the tempting fix (widening HREF_SEGMENT to admit %xx, which is what the acceptance run patched locally) would weaken every literal href in order to fix one path core can spell for itself, and a test asserts it stays strict.

web-ui derives the href from pluginId rather than validating the transmitted string. That is stronger than validation: the only encoded path the shell can emit is one it computed itself from a charset-checked id, so a version skew or a compromised control plane still cannot inject an arbitrary encoded path into the trusted header. It reuses isValidPluginId from C8b rather than adding a third copy of the pattern.

uiRouteCatalog.ts restates the manifest's plugin-id gate rather than importing it, because manifestLoader.ts keeps those declarations in the exact source form C8b's parity test anchors on and exporting them would reformat the matched lines. The restatement is pinned the same way — a test reads manifestLoader.ts and asserts both are character-identical, so drift fails a test rather than a nav entry.

plugin-api 1.2.0 → 1.3.0

MINOR, additive. CHANGELOG entry written, golden snapshot regenerated deliberately — the whole diff is six lines:

Symbol Change
ServicesAccessor.getOptional added
UiNavEntryInput.pluginUi added (true | undefined)
ResolvedUiNavEntry.pluginUi added
UiNavEntryInput.href widened to optional
UiNavEntry now extends Omit<UiNavEntryInput, 'href'> with href: string required — the kernel resolves pluginUi to a concrete path at registration

Mutation check

Every claim has a test that reddens when the fix is reverted:

Mutation Reddens
Drop optional_requires from declaredServiceNames 4
Make walkCapabilityInstallChain read optional entries 2
Route a pluginUi entry through assertInAppHref 3
Make web-ui trust the transmitted href 2
Re-attach the core ledger to ANTHROPIC_API_KEY 2

The pg suite runs with every provider key stripped from the environment, so a regression that reattaches the ledger to a credential fails here rather than in staging. It asserts the ledger count against the migration directory rather than a hardcoded 47, and names plugin_public_path_grants and plugin_sql_grants individually — "the ledger ran" and "consent is recordable" are the two separate claims #796 makes.

Verification

  • middleware: build, typecheck, typecheck:test (ratchet held at 406), lint (0 errors), 7880/7880 tests with Postgres, 0 fail
  • web-ui: lint (0 errors, 47 pre-existing warnings), typecheck, 785/785 vitest, production build
  • check-core-decoupling.mjs: 3300 → 3299

Two of that reduction came from rewording installServiceActivationTruthful.test.ts (#799), which merged carrying two un-baselined dev-platform references — origin/main measured 3302 against its own committed baseline of 3300 before this branch. Reworded rather than raised; both are fixture strings with no behavioural role, and the test still passes 3/3.

Merged origin/main (#793 + #799). #793 landed the scoped-id fix for the plugin-UI host page and extracted web-ui/app/_lib/pluginId.ts, so the redundant half of this branch's original page change was dropped and the shared, parity-pinned helper reused instead.

Not merging — READY for review.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Cross-family review (Forge)

GPT-5.4 at reasoning_effort=high via codex exec, plus an independent trace of the same
control flow. Deliberately not Claude-family: this PR was authored by an Anthropic-family model
and the point of the gate is to not share its blind spots.

Two findings. One is a real regression this PR introduces; it is fixed in this branch. The
other is a claim in the PR body that the tests did not actually support; it is now supported.

Finding 1 — boot-crash on a contended migration lock (HIGH, fixed)

Moving the ledger out of activate() also moved it out from behind a catch, and the migrator's
timeout budget did not move with it.

runMultiOrchestratorMigrations waits MULTI_ORCH_MIGRATION_LOCK_WAIT_MS = 2000 for the advisory
lock, re-reads the ledger, and — if work is still owed — throws
(registry/migrator.ts:113-120). That 2s was sized for its old call site, and the wording was
picked to match a transient-error regex:

  • old path: throw lands inside activate()activateAllInstalled catches per plugin, logs,
    calls markActivationFailed, boot continues (toolPluginRuntime.ts:251-264), and
    TRANSIENT_ACTIVATION_ERROR_RE in bootstrap.ts:197 matches /timed out/i so the plugin is
    retried on the next boot.
  • new path: throw lands at top level in main()main().catchprocess.exit(1)
    (index.ts:5376-5378).

So a cold multi-replica boot — 47 files to apply, and "the winner finishes inside 2s" is not a
contract — turned a survivable, self-healing race into a crash. That is the opposite direction
from what #796 is for, and this epic already tracks multi-replica migrator racing as a live
concern (specs/470-dev-platform-plugin/implementation.md:35).

Fixed in coreMigrations.ts: lock contention specifically is retried, bounded at 60s, with a
500ms spacing. Each attempt re-enters the migrator, which re-reads the ledger — so once the winner
commits, the next attempt takes the migrator's own "applied by another replica while waiting" path
and returns clean. Everything else still propagates on the first occurrence: a broken SQL file must
fail loudly, which is the whole point of the issue. Past the budget the original error is rethrown
unchanged.

The predicate matches the migrator's message because the migrator exports no error type. That
string coupling is pinned, not trusted: a test reads migrator.ts and fails if the phrase or
LOCK_KEY stops being produced.

Finding 2 — the ANTHROPIC_API_KEY mutation claim was not covered (MEDIUM, fixed)

The mutation table claims re-attaching the ledger to ANTHROPIC_API_KEY reddens 2 tests. It does
not. coreMigrations.pg.test.ts calls runCoreMigrations directly and never exercises
index.ts, so reverting the boot wiring — deleting the call, or putting it back behind a provider
key — leaves that suite fully green. The regression #796 is actually about lives in the wiring, and
nothing asserted the wiring. The pg suite proves the function; it structurally cannot see its caller.

Added middleware/test/coreMigrationsBootWiring.test.ts, which pins the two properties the pg suite
cannot: that runCoreMigrations is awaited before activateAllInstalled(), and that the call
is not gated on a provider credential. Source-reading, in the same style as this PR's own plugin-id
parity pin — importing index.ts would boot the middleware.

Mutation check on the new guards

Mutation Result
Remove the contention retry (propagate instead) 4 tests redden
Move runCoreMigrations after activateAllInstalled 4 tests redden
Break the pinned contention phrase 7 tests redden
Restore 8/8 green

Confirmed against the code (no change needed)

  • P0: core's migration ledger only runs when an LLM key is set — C4 and C7 grant tables never exist (#470 P5) #796 orderingindex.ts:1717 precedes :1733. runCoreMigrations returns 'no-database'
    and does not throw when DATABASE_URL is unset, so a pg-less deployment degrades with a log.
    Pool closed in finally on every path, and end() cannot mask a migration error.
  • No double-apply. Both callers go through the same runMultiOrchestratorMigrations, the same
    _multi_orchestrator_migrations ledger and the same advisory lock (ns 4410). The migrator
    re-reads pending state under the lock (migrator.ts:122-126), so the pre-lock read is a fast
    path, not a decision. Crash-on-migration-failure also matches the existing runAuthMigrations
    precedent.
  • requires: has no optionality, and two independent gates enforce it — optional capabilities are unrepresentable (#470 P5) #795getOptional delegates to the gated get, so an undeclared name still throws
    ServiceNotDeclaredError; a typo cannot become undefined. requires semantics unchanged.
    resolveCapabilities and walkCapabilityInstallChain both ignore optional_requires.
  • C7 not bypassedoptional_requires: ["graphPool@^1"] satisfies the declaration gate only.
    assertSqlAccess still demands permissions.sql + an operator grant, and the pool is still
    borrowed rather than handed over (pluginContext.ts:313-330). Legacy allowlist unaffected.
  • No scoped plugin id can express a valid nav href — SPA route needs percent-encoding, validator forbids it (#470 P5) #798 — href built by core only; HREF_SEGMENT stays strict and a test asserts it.
    href/pluginUi mutually exclusive, non-true pluginUi rejected. web-ui re-derives from
    pluginId and discards the transmitted href; isValidPluginId admits only lowercase npm-style
    ids, so encodeURIComponent of a passing id cannot produce a traversing or scheme-bearing path.
    The ENCODABLE_PLUGIN_ID restatement is genuinely character-identical to manifestLoader.ts:183.

Ratchet — confirmed by measurement

origin/main really is red: exit 1, 3300 → 3302, middleware/test 1030 → 1031 (#799 landed
two un-baselined references). This branch exits 0 at 3299. The reduction is reworded fixture
strings, not a raised baseline — so this PR also returns a currently-failing required check to green.

Finding 3 — the full-suite run is intermittently flaky in one dev-platform pg file (LOW, pre-existing, not fixed here)

The PR reports "7880/7880 tests with Postgres, 0 fail". That holds on some runs and not others:

Run Result
Full suite, with the fix 7918/7921, 1 faildevJobTaskStore.pg.test.ts "surfaces a real event tail through the seam" (4 !== 2)
Full suite, fix stashed (baseline) 7913/7916, 1 fail — same file, different test: "FENCES a mismatched lease"
Full suite, after merging main 7926/7928, 0 fail
That file in isolation 15/15 pass
npm run test:pg (concurrency 1) 343/343 pass, 0 fail — every run

A different test failing in the same file on each run, and passing on a third, is nondeterministic
pollution rather than a regression — and it reproduces with all of my changes stashed, so it is not
introduced here. The mechanism is visible in the scripts: npm test globs test/**/*.test.ts,
which includes *.pg.test.ts, and runs at --test-concurrency=4 against the shared test
database; npm run test:pg runs the same files at concurrency 1 and is green every time. Every pg
suite is therefore executed twice, once under contention.

Left alone deliberately — it is the repo's known test-pollution issue and not this PR's job.
Flagged because a green full-suite run here is luck rather than evidence, and because the affected
file is itself dev-platform code that epic #470 is extracting.

Verification (re-run after the fix, and again after merging main)

origin/main moved to ac491471 (#800) mid-review. Merged it; #800 had independently reworded
installServiceActivationTruthful.test.ts for this same ratchet, so the conflict was resolved in
main's favour — it neutralizes the tool name to acme_job_start and explains why, which
reduces references further than this branch's rewording did.

Final state (289ac56f), all green:

  • middleware: build ✓, typecheck ✓, typecheck:test ✓ (ratchet held at 406), lint ✓ 0 errors
  • middleware tests: npm test 7926/7928 pass, 0 fail (2 skipped);
    npm run test:pg 343/343 pass, 0 fail against 127.0.0.1:55438
  • new tests: 5 added, 8/8 green, and all three mutations redden
  • web-ui: typecheck ✓, 795/795 vitest, lint ✓ 0 errors / 49 pre-existing warnings
  • check-core-decoupling.mjs: exit 0, held at 3299 (still green after the merge)

Verdict: MERGE. The three fixes are correct, and the reasoning in the comments is unusually
load-bearing — the optional_requires topo-sort omission and the deliberately-unchanged
HREF_SEGMENT are both the right call and both argued rather than asserted. The one real defect
was a context assumption that did not survive the move it was part of, which is exactly the class
a same-family reviewer is least likely to question.

Weegy added 4 commits August 20, 2026 20:29
Each was measured against a real core while installing the first extracted
plugin, not inferred. Each failed in a way core could not see.

#795 — `requires:` had no optionality, and two gates enforced it.

C2b makes `ctx.services.get(name)` throw for a name in neither `requires:`
nor `provides:`, so a plugin MUST declare anything it might resolve. The
installer and the boot loop then treat every `requires:` entry as a hard
prerequisite. A plugin with a degradable dependency was therefore
unrepresentable: declaring it blocked the install, omitting it made
resolution throw.

New manifest field `optional_requires:`, same capability-ref syntax. It
satisfies the declaration gate and neither enforcement gate — no
`install.missing_capability`, no activation hold, no provider demanded.
`ctx.services.getOptional(name)` is the accessor that says so at the call
site; it is declaration-gated exactly like `get`, because a typo must not
quietly become `undefined`.

The ordering consequence is deliberate and documented rather than papered
over: an optional dependency contributes no topo-sort edge, so an optional
provider that IS installed may activate after its consumer. An edge from a
link the kernel may not enforce would turn a mutual optional reference into
a cycle — a boot failure caused by a dependency declared as skippable. The
contract says to resolve optional services lazily instead.

Surfaced on the install DTO (`Plugin.optional_requires`, and the registry
teaser) so the consent UI can render these as optional rather than as
blockers. `pluginServiceGrants.ts` loses the paragraph calling this an open
design question — every remaining legacy-allowlist row now has a manifest
fix available.

#796 — the core migration ledger only ran when an LLM key was configured.

`middleware/migrations/` is core's own directory, 47 files including C4's
`plugin_public_path_grants` and C7's `plugin_sql_grants`. Its only
production caller was inside harness-orchestrator's `activate()`, several
hundred lines past an early return taken whenever no provider resolves. On
a deployment with no key, core had no schema at all — not a degraded one,
none — so recording either operator consent was structurally impossible.
Silent by construction: nothing logged a migration error because no
migration was ever attempted.

`platform/coreMigrations.ts` now applies it at boot, before any tool plugin
activates and independent of every provider. Ordering is load-bearing:
`ToolPluginRuntime` reads a plugin's SQL-grant row while building its
context, so the grant tables must exist by then. It opens its own
short-lived connection from `DATABASE_URL` rather than waiting for
`graphPool`, which the knowledge-graph plugin publishes during activation —
waiting for it would reintroduce the same defect one layer up. The
orchestrator's call is retained as an explicit second pass (one SELECT
against a current ledger) and can never be the only caller again.

#798 — no scoped plugin id could express a nav href.

Core serves a plugin's bundled UI at `/p/:pluginId/ui/` and Express splits
on a raw `/`, so `@scope/name` resolves only percent-encoded — measured
encoded 200, raw 404. `HREF_SEGMENT` rejects `%` for a good reason: the
shell decides "core destinations win" by string equality, which
percent-encoding defeats. The only URL that worked was the only one the
validator refused.

`registerNav` now accepts `pluginUi: true` in place of a literal href and
renders the canonical path itself from the id it already holds, so a plugin
never hand-builds an encoded href. The literal-href validator is untouched
— the tempting fix (widening `HREF_SEGMENT` to admit `%xx`, which the
acceptance run patched locally) would weaken every literal href to fix one
path core can spell for itself, and a test asserts it stays strict.

web-ui derives the href from `pluginId` rather than validating the
transmitted string. That is stronger than validation: the only encoded path
the shell can emit is one it computed itself from a charset-checked id, so
a version skew or a compromised control plane still cannot inject an
arbitrary encoded path into the trusted header. It reuses `isValidPluginId`
from C8b rather than adding a copy of the pattern.

`uiRouteCatalog.ts` restates the manifest's plugin-id gate rather than
importing it, because `manifestLoader.ts` keeps those declarations in the
exact source form C8b's parity test anchors on. The restatement is pinned
the same way — a test reads `manifestLoader.ts` and asserts both are
character-identical, so drift fails a test rather than a nav entry.

plugin-api 1.2.0 → 1.3.0 (MINOR, additive), CHANGELOG entry, golden
snapshot regenerated deliberately: `getOptional`, `UiNavEntryInput.pluginUi`,
`ResolvedUiNavEntry.pluginUi`, and `UiNavEntryInput.href` widened to
optional. `UiNavEntry.href` stays required — the kernel resolves `pluginUi`
to a concrete path at registration.

Mutation-checked, five mutations each reddening the tests that claim the
behaviour: dropping `optional_requires` from `declaredServiceNames` (4
fail), making the install-chain walk read optional entries (2), routing a
`pluginUi` entry through `assertInAppHref` (3), making web-ui trust the
transmitted href (2), and re-attaching the core ledger to `ANTHROPIC_API_KEY`
(2). The pg test runs with every provider key stripped from the environment,
so a regression reattaching the ledger to a credential fails here rather
than in staging.

Verified: middleware build, typecheck, typecheck:test (ratchet held at 406),
lint (0 errors), 7880/7880 tests with pg; web-ui lint (0 errors, 47
pre-existing warnings), typecheck, 785/785 vitest, build.

Core-decoupling ratchet 3300 → 3299. Two of that came from rewording
`installServiceActivationTruthful.test.ts` (#799), which merged carrying two
un-baselined dev-platform references — main measured 3302 against its own
3300 baseline before this branch. Reworded rather than raised.

Fixes #795, #796, #798
Moving `middleware/migrations/` out of the orchestrator plugin's `activate()`
also moved it out from behind a catch, and the migrator's lock budget did not
move with it.

`runMultiOrchestratorMigrations` waits 2s for the `_multi_orchestrator_migrations`
advisory lock, re-reads the ledger, and throws if work is still owed. That budget
was sized for its old call site: inside `activate()`, capped at 10s by
ToolPluginRuntime, where `activateAllInstalled` caught the throw per plugin,
marked that one plugin errored and let boot continue — and where the "timed out"
wording was chosen so `bootstrap.retryErroredPlugins` would classify it as
transient and re-attempt on the next boot.

At boot there is no such catch. The throw reaches `main().catch` and becomes
`process.exit(1)`, so a cold multi-replica boot with 47 files to apply turned a
survivable, self-healing race into a crash loop.

Lock contention specifically is now retried, bounded at 60s. Each attempt
re-enters the migrator, which re-reads the ledger, so once the winner commits the
next attempt takes the migrator's own "applied by another replica while waiting"
path and returns clean. Every other error still propagates on its first
occurrence — a broken SQL file must fail loudly, which is the point of #796. Past
the budget the migrator's error is rethrown unchanged.

The predicate matches the migrator's message because it exports no error type;
that coupling is pinned by a test that reads migrator.ts rather than trusted.

Also adds the boot-wiring test the pg suite structurally cannot provide.
`coreMigrations.pg.test.ts` calls `runCoreMigrations` directly, so reverting the
`index.ts` wiring — deleting the call or putting it back behind a provider key —
left it fully green. The regression #796 is about lives in the wiring. The new
test pins that `runCoreMigrations` is awaited before `activateAllInstalled()` and
is not gated on a credential.

Mutation check: removing the retry reddens 4, moving the call after
`activateAllInstalled` reddens 4, breaking the pinned phrase reddens 7.
#800 reworded installServiceActivationTruthful.test.ts for the same #470
ratchet this branch touched. Took main's side on both hunks: it neutralizes
the tool name to 'acme_job_start' and says why, which reduces dev-platform
references further than this branch's rewording did.
@Weegy
Weegy merged commit d0a6469 into main Aug 20, 2026
9 checks passed
Weegy added a commit that referenced this pull request Aug 20, 2026
…ansaction (C11)

A witness is SQL the PLUGIN writes and CORE executes on the connection that
holds the handoff transaction and its advisory lock. It ran as
`client.query(text)` with no bind values, which node-postgres sends over the
SIMPLE query protocol — and that protocol accepts multiple commands per
message. Measured against PostgreSQL 16.13:

    SELECT true AS ok; COMMIT; DROP TABLE victim

ran all three. The COMMIT ended the seeder's transaction, so the advisory lock
was released mid-seed, `dryRun`'s closing ROLLBACK became a no-op, and the DROP
committed. `DELETE FROM _multi_orchestrator_migrations RETURNING true` needed
no semicolon at all: one row, one column, one boolean, so it passed the shape
check and deleted the donor rows this module documents as never deleted — the
rollback path for the whole extraction.

Two guards, both enforced by PostgreSQL rather than by scanning the string:

  - each witness runs inside SAVEPOINT + `SET LOCAL transaction_read_only = on`,
    so every write is refused; ROLLBACK TO SAVEPOINT is the way back, because
    the GUC cannot be cleared once a statement has taken a snapshot
  - `queryMode: 'extended'` forces the extended protocol, where the server
    refuses multi-command strings outright

Neither is a semicolon scan: a semicolon inside a legal string literal is still
a valid witness, and a test pins that.

The witness type becomes SQL text rather than a callback. A callback cannot be
fenced — it receives the privileged client — and the shape check existed twice,
once in the accessor and once in the CLI, each with the same hole. Both copies
are deleted; the seeder owns execution, so neither caller can get it wrong.

Also: teardown moved out of `finally` (a throw there discards the witness error
that caused it), plugin-api re-cut as 1.4.0 because #802 took 1.3.0 on main,
and PG_TEST_FLOOR 264 -> 270 for the six new pg tests.

Mutation check: reverting the fence to `client.query(sql)` fails exactly the
five hardening tests and nothing else.
Weegy added a commit that referenced this pull request Aug 21, 2026
…sql.seedLedger (epic #470 C11) (#806)

* chore(#470): delete the dev-platform backend tree, routers and tests (C10)

Epic #470 C10 — the flip. The Dev Platform now lives in
byte5ai/omadia-dev-platform and installs via Hub/ZIP.

Removed:
- middleware/src/devplatform/ (62 files) — stores, worker, backends, LLM
  proxy, pipeline/gates, GitHub App, triggers, routers, wireDevPlatform
- middleware/test/devplatform/ (58 files)
- middleware/scripts/dev-transcript.ts

Adversarial eval (#498): the `brief_delimiter` Tier A probe ran the real
`composeBrief` out of src/devplatform/. A probe against a module core no
longer ships measures a library, not a deployed defense — so the probe, its
five corpus scenarios (direct-injection.jsonl, indirect-injection.jsonl) and
their baseline rows leave with it. This is a real coverage reduction and is
recorded as such in test/adversarial/README.md, with both ways to close it.
Deterministic corpus: 12 scenarios -> 7.

* chore(#470): delete the dev-runner shim, sidecars and compose topology (C10)

Removed:
- middleware/packages/dev-runner-shim/ (23 files) — the in-container agent
  shim. Never built by `npm run build`, yet index.ts resolved its dist/ at
  runtime; the extraction removes that inconsistency with it.
- middleware/sidecars/dev-runner/ (2), dev-runner-daemon/ (30, dockerode),
  dev-dind/ (2)
- docker-compose.dev-platform.yaml

All four now live in byte5ai/omadia-dev-platform, which owns their GHCR
publishing, SBOM and signing pipeline.

* chore(#470): unwire the dev platform from index.ts, config and env (C10)

index.ts (-238 lines):
- 15 devplatform imports + the now-dead DeviceFlowStore / ConductorRoleStore
- the GitHub-webhook block mounted before express.json
- the whole assembly block: assembleDevPlatform / mountDevPlatform, the
  GitHub-App routers, the three chat orchestrator tools, worker start +
  SIGTERM/SIGINT hooks, the uiRouteCatalog nav registration, the retention
  cron and the no-graphPool warning

Side effect worth calling out: the `/api/v1/dev-runner/llm/` carve-out sat
BEFORE the conductor inbound webhook router, so an express.json ran ahead of
that router's route-level express.raw() and short-circuited it (body-parser
marks the request `_body`). Deleting the carve-out restores the order the
surrounding comments already document: pluginRawBodyMount -> conductor raw
router -> global express.json.

config.ts (-305 lines): all 43 DEV_*/FLY_* dev-platform schema keys, the
`devPlatform` namespace, `buildDevPlatformConfig`, `csvList`, and the
CORE_DEV_PREFIXED_KEYS / DevPlatformEnvKey / isDevPlatformEnvKey machinery
that existed only to hold those keys out of the top level — `Config` is now
`ParsedConfig`. `devPlatformBootRefusals` goes too: the plugin owns both
interlocks (pluginConfig.ts, verified), so they became activation refusals
rather than vanishing. KEPT: devFlag() (two PUBLIC_MCP_* call sites),
DEV_ENDPOINTS_ENABLED, DEV_ENDPOINTS_LOOPBACK_ONLY, FLY_APP_NAME.

.env.example: the 15-key block replaced with a pointer to the plugin repo.

middleware/package.json: the §5 note — express/pg/zod stay as plugin
peerDependencies resolved through the host node_modules symlink.

* chore(#470): delete the dev-platform web-ui surface and its i18n keys (C10)

- web-ui/app/admin/dev-platform/ (29 files) — the operator SPA. It ships from
  the plugin now as a compiled Vite bundle served through the C8 static host
  at /p/<pluginId>/ui/.
- web-ui/app/_components/devjobs/ (6 files) + app/_lib/useDevJobEvents.ts
- app/admin/page.tsx — the grid card. The generic `requiresNavFrom` mechanism
  stays; this was its only user.
- app/_lib/i18n-structural.test.ts — the GateInbox.tsx path entry.

H3 resolved by omission, as the plugin's ACCEPTANCE-RUN records. chat/page.tsx
no longer special-cases `tool.name === 'dev_job_start'`; a dev-job start from
the installed plugin now falls through to the generic long-running-task card
(`isTaskStartToolName` / TaskChatCard), which is the accepted degradation.

i18n: 299 leaf keys per locale removed (adminDevPlatform.* 288, chat.devJob.* 9,
admin.index.cards.devPlatform.* 2) = 598 across en+de, plus the four orphaned
`i18n-identical-allowlist.json` entries the validator flagged.
`npm run i18n:check` OK — 3560 keys; `i18n:literals` translate=0.

* chore(#470): drop the dev-runner CI matrix and supply chain from core (C10)

publish-images.yml:
- the `dev-runner` and `dev-runner-daemon` matrix entries
- the runner-image supply chain: cosign install, syft SPDX-JSON SBOM,
  keyless sign + attest (all guarded on `matrix.name == 'dev-runner'`)
- `id-token: write`, which existed solely for that keyless signing

auto-release.yml / release.yml: the matching caller-side `id-token: write`
grants, which a reusable workflow cannot self-grant and nothing else needs.
The `if: false` npm-provenance job in release.yml keeps its own grant.

byte5ai/omadia-dev-platform owns runner GHCR publishing, SBOM and signing
now. Per implementation.md §2.4 the cosign certificate identity binds to
repo + workflow + ref, so the new signer needs the transition
`--certificate-identity-regexp` landed before it publishes — that is P4's
job in the plugin repo, not core's.

The missing-Dockerfile guard stays: it also computes the version `stamp`
every remaining image consumes; only its dev-runner-daemon justification
comment is gone.

scripts/wave-{implement,verify}.workflow.mjs: the generic wave prompts baked
in "terminal transitions go through finalizeDevJob" — a dev-platform domain
rule in a domain-agnostic prompt, naming a function core no longer ships.
Genericised to "the subsystem's single finalizer"; the rule survives, the
implementor name does not. Same for the `docs/dev-platform/` example path.

* fix(#470): restore four core config keys the C10 cut swallowed (C10)

`tsc` caught these, which is exactly why C10 has to be one PR.

Three LONG_RUNNING_* keys sat INTERLEAVED with the dev-platform keys in
config.ts, between DEV_PLATFORM_RUNNER_BASE_URL and DEV_PLATFORM_CLI_BIN, and
went out with the block:

  LONG_RUNNING_SUBAGENT_TOOLS   — W2-2 / issue #543, the generic
  LONG_RUNNING_TASK_STALE_MS      `<tool>_start/_status/_list` seam that any
  LONG_RUNNING_TASK_RETAIN_MS     slow tool opts into. Not dev-platform.

FLY_APP_NAME went too, despite CORE_DEV_PREFIXED_KEYS documenting in prose that
it "describes the host, not the feature, so it must survive the extraction".

All four are restored under an explicit heading that says why they are core, so
the next person reading config.ts does not have to re-derive it. Also drops the
`node:os` import, now unused (it backed the workspace-dir default), and rewords
the three comments this PR itself introduced that named the extracted subsystem
— C10 must not add coupling references while removing them.

Verified: middleware build + typecheck + adversarial/golden tsconfigs green;
6925 pass / 0 fail; typecheck:test ratchet 406 -> 371 (baseline lowered);
lint 0 problems. web-ui typecheck green, 717 pass / 0 fail, i18n 3560 keys OK.

* docs(#470): record the C10 flip — ratchet baseline, specs, changelog (C10)

Ratchet: **3,300 → 214**, updated consistently in all three places the README
says must agree — `decoupling-baseline.json`, the README "Baseline **214**"
line, and the `acceptance.md` guard row.

Nine of fourteen zones read CLEAN. Every survivor is scheduled, not stranded:

  migrations       69  C11 — 0022-0030 stay; core still applies them
  middleware/test  62  C13 — fixture strings + the legacy-key regression test
  scripts          27  C13 — the ratchet's own pattern list (needs a
                            self-exclusion before the total can reach 0)
  middleware/src   19  C12 — publicPaths (6); rest are comments
  web-ui/app       19  C13 — nav test fixtures, two comments
  packages         17  C13 — plugin-api CHANGELOG recording the removal
  env-example       1  the plugin repo URL, which cannot be reworded

Also: `test-typecheck-baseline.json` 406 → 371, and `package-lock.json`
regenerated (npm left `packages/dev-runner-shim` as an `extraneous: true`
stanza rather than dropping it; `npm ci` verified clean afterwards).

Docs:
- README gains a "C10 — the flip" status section: what left, what was kept and
  why, H3 resolved by omission, the adversarial coverage reduction, and the
  express.json ordering bug the deletion fixed. The H3 "decision before code"
  is struck through — it is answered now.
- plan.md §4.2 gains a C10 note (and flags that its `devJobStepEffect.ts` line
  went stale in the other direction: C5 deleted that port as dead code). P4 row
  marked shipped.
- docs/CHANGELOG.md: "Dev Platform moved to byte5ai/omadia-dev-platform
  (install via Hub/ZIP)" — written for operators, so it leads with how to get
  it back, states that no data is touched, and that in-flight jobs survive the
  upgrade because C12 has not run yet.

* feat(#470): migration handoff — seed a plugin ledger on witnesses, not trust (C11)

A plugin extracted out of core inherits installations whose schema core already
created, recorded in a core ledger the plugin cannot see. Its own ledger is
empty, so its migration runner would re-apply every file.

The naive handoff copies the donor rows and skips those files. That is correct
on a healthy database and silently destroys one specific installation: rows
present, tables ABSENT — a restore from an older snapshot, a version-skewed
rollback, an operator who dropped a table during an incident. The plugin
activates green and every request 500s, nine steps behind the cause.

So the donor ledger is corroboration and a per-file WITNESS is the decision:
one catalog query that is true only when the schema object that file creates is
actually there.

- `platform/pluginMigrationHandoff.ts` — `seedPluginLedgerFromDonor`. One
  transaction under C7's advisory lock and namespace, keyed on the ledger, so a
  seed serialises against a migrate and against another seed. `dryRun` rolls the
  whole transaction back, including the ledger DDL and anything a witness
  touched. Reports `skippedNoWitness` — the files the donor recorded whose
  witness is false — because that number is the restore alarm. Contains no
  DELETE: the donor rows are the rollback path.
- `ctx.sql.seedLedger({ entries, dryRun })` — the plugin supplies filenames and
  witness SQL, core supplies the donor ledger. Matching is by filename STEM, so
  a codegen'd `0022_x.js` adopts core's `0022_x.sql`. Witness SQL rather than a
  callback: a callback cannot be printed, and the point of `dryRun` is that an
  operator reads which query proved which file.
- `pluginLedgerDdl` / `migrationChecksum` / `listMigrationFiles` are now exported
  from `pluginMigrations.ts` and used by the seeder, so a seeded row is
  byte-identical to one the runner would have written. A wrong checksum would
  trip the drift guard one boot later.
- `@omadia/plugin-api` 1.3.0 (additive; `seedLedger` is optional, so a plugin
  still activates against a 1.2.0 core). Snapshot regenerated.
- `middleware/scripts/plugin-ledger-handoff.mjs` — operator CLI, dry run by
  default, `--apply` the only way to write. Named and written generically: core
  may not name the extracted plugin, and the next plugin to leave wants it
  unchanged.

Tests: 27 new (25 pg-gated). Six required scenarios plus the counter-proof, a
wiring suite through `createPluginContext`, and a source-level assertion that
the donor ledger name still matches the migrator that creates it. Mutation
check: replacing the witness gate with `donorHasIt ||` fails exactly the three
tests that assert it, case (b) included.

Ratchet held at 214.

* ci(#470): re-pin the Postgres coverage floor to the measured 264 (C11)

The floor was 269 and describing a tree that no longer exists. C10 deleted the
Dev Platform and its 26 pg tests left with the code they measured, so that
branch runs `ran=243` and goes red on a guard that is supposed to catch SILENT
skips, not deliberate deletions. A coverage floor that outlives its suites is
not protecting anything; it is just failing.

Two movements, both read off CI rather than guessed:

  -26  C10's delete
  +21  C11's `pluginMigrationHandoff.pg.test.ts` (14) and
       `pluginMigrationHandoffAccessor.pg.test.ts` (7)

243 + 21 = 264, and CI reports exactly 264 — which is simultaneously the check
that nothing else went quiet on the way. Deliberate removal of the covered code
is the one legitimate reason to lower this floor; every other drop is the #565
silent skip it exists to catch, and the comment now says so.

* fix(#470): fence plugin-supplied witness SQL inside a read-only subtransaction (C11)

A witness is SQL the PLUGIN writes and CORE executes on the connection that
holds the handoff transaction and its advisory lock. It ran as
`client.query(text)` with no bind values, which node-postgres sends over the
SIMPLE query protocol — and that protocol accepts multiple commands per
message. Measured against PostgreSQL 16.13:

    SELECT true AS ok; COMMIT; DROP TABLE victim

ran all three. The COMMIT ended the seeder's transaction, so the advisory lock
was released mid-seed, `dryRun`'s closing ROLLBACK became a no-op, and the DROP
committed. `DELETE FROM _multi_orchestrator_migrations RETURNING true` needed
no semicolon at all: one row, one column, one boolean, so it passed the shape
check and deleted the donor rows this module documents as never deleted — the
rollback path for the whole extraction.

Two guards, both enforced by PostgreSQL rather than by scanning the string:

  - each witness runs inside SAVEPOINT + `SET LOCAL transaction_read_only = on`,
    so every write is refused; ROLLBACK TO SAVEPOINT is the way back, because
    the GUC cannot be cleared once a statement has taken a snapshot
  - `queryMode: 'extended'` forces the extended protocol, where the server
    refuses multi-command strings outright

Neither is a semicolon scan: a semicolon inside a legal string literal is still
a valid witness, and a test pins that.

The witness type becomes SQL text rather than a callback. A callback cannot be
fenced — it receives the privileged client — and the shape check existed twice,
once in the accessor and once in the CLI, each with the same hole. Both copies
are deleted; the seeder owns execution, so neither caller can get it wrong.

Also: teardown moved out of `finally` (a throw there discards the witness error
that caused it), plugin-api re-cut as 1.4.0 because #802 took 1.3.0 on main,
and PG_TEST_FLOOR 264 -> 270 for the six new pg tests.

Mutation check: reverting the fence to `client.query(sql)` fails exactly the
five hardening tests and nothing else.

* chore(plugin-api): 1.5.0 on top of main's 1.4.0; PG_TEST_FLOOR 252 → 279 for the C11 pg suites
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

requires: has no optionality, and two independent gates enforce it — optional capabilities are unrepresentable (#470 P5)

1 participant